Overview fully SSR#175
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThis pull request refactors data retrieval and component interfacing for the dashboard. Server-side actions and custom React hooks for fetching framework categories, organization controls, and progress are removed. In their place, new asynchronous functions and utility methods are introduced to fetch frameworks, control data, and compute control statuses. Component props and type definitions are updated to pass pre-fetched data, eliminating redundant hooks. Additionally, the page components have been enhanced with authentication and organization checks, ensuring that only valid sessions proceed. Error handling and redirection logic have also been improved. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant FP as FrameworkPage
participant Auth as auth()
participant GF as getFramework()
participant GFC as getFrameworkCategories()
participant FO as FrameworkOverview
U->>FP: Request framework page
FP->>Auth: Retrieve session
Auth-->>FP: Session with organizationId
FP->>GF: Fetch framework (frameworkId, orgId)
FP->>GFC: Fetch framework categories (frameworkId, orgId)
FP->>FO: Render view with organizationFramework and organizationCategories
sequenceDiagram
participant U as User
participant CP as SingleControlPage
participant Auth as auth()
participant GC as getControl()
participant GCP as getOrganizationControlProgress()
participant SC as SingleControl
U->>CP: Request control page
CP->>Auth: Retrieve session
Auth-->>CP: Session info
CP->>GC: Fetch control data (id)
GC-->>CP: Organization control details
CP->>GCP: Fetch progress data
GCP-->>CP: Control progress metrics
CP->>SC: Render SingleControl with control data & progress info
Possibly related PRs
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (12)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/data/getOrganizationControlProgress.ts (1)
18-88: Good implementation with robust progress calculation logicThe function correctly fetches control requirements and calculates detailed progress metrics. The approach of using a structured type for the response and systematic calculation of completed items by type is well-implemented.
Consider adding error handling for database operations to improve robustness:
export const getOrganizationControlProgress = async (controlId: string) => { const session = await auth(); if (!session) { return { error: "Unauthorized", }; } + try { const requirements = await db.organizationControlRequirement.findMany({ where: { organizationControlId: controlId, }, include: { organizationPolicy: true, organizationEvidence: true, }, }); // Rest of the function remains the same return { data: { progress, }, }; + } catch (error) { + console.error("Error fetching organization control progress:", error); + return { + error: "Failed to fetch control progress", + }; + } };apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/data/getControl.ts (1)
4-30: Secure implementation with good data fetching logicThe function correctly validates user session and fetches organization control data with appropriate relations. The security check ensures users can only access controls from their organization.
Similar to the progress function, consider adding error handling for database operations:
export const getControl = async (id: string) => { const session = await auth(); if (!session) { return { error: "Unauthorized", }; } + try { const organizationControl = await db.organizationControl.findUnique({ where: { organizationId: session.user.organizationId, id, }, include: { control: true, OrganizationControlRequirement: { include: { organizationPolicy: true, organizationEvidence: true, }, }, }, }); return organizationControl; + } catch (error) { + console.error("Error fetching organization control:", error); + return { + error: "Failed to fetch control data", + }; + } };apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/page.tsx (1)
28-39: Consider simplifying the default progress data extractionThe condition for extracting progress data is quite complex. Consider simplifying it for better readability:
- const progressData: ControlProgressResponse = ("data" in - (organizationControlProgressResult || {}) && - organizationControlProgressResult?.data?.progress) || { - total: 0, - completed: 0, - progress: 0, - byType: {}, - }; + const progressData: ControlProgressResponse = + organizationControlProgressResult?.data?.progress || { + total: 0, + completed: 0, + progress: 0, + byType: {}, + };The simplified version works because if
organizationControlProgressResultis falsy or doesn't have the expected structure, it will fall back to the default object.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/data/getFramework.ts (1)
3-21: Consider handling the case when no framework is found.Right now, if
findUniquereturnsnull, the function simply returnsnullwithout explanation. This could propagate errors elsewhere in the application. You might want to throw or return a more descriptive error or default object to handle the scenario gracefully when a framework does not exist for the given IDs.export const getFramework = async ( frameworkId: string, organizationId: string ) => { const framework = await db.organizationFramework.findUnique({ /* ... */ }); - return framework; + if (!framework) { + throw new Error(`Framework not found for frameworkId=${frameworkId} and organizationId=${organizationId}`); + } + return framework; };apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/data/getFrameworkCategories.ts (1)
3-36: Ensure empty result scenarios are handled.Returning an empty array from
findManyis valid, but if your downstream logic expects categories to exist, it may cause confusion. Consider verifying whether the returned array is empty and handling that case accordingly, for example by returning a default or throwing an error to make dependencies more robust.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/page.tsx (2)
19-23: Consider providing user feedback prior to redirecting.When the session is missing, the code executes an immediate
redirect("/"). Providing a brief message or an alert can help users understand why they've been redirected.
35-39: Verify the presence of required categories.If
getFrameworkCategoriesreturns an empty array or undefined, consider how that affects your page rendering. You may want to display a helpful message on the UI instead of silently passing an empty array.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkControls.tsx (3)
3-10: Consolidate imports if they are reused in multiple files.All of these types come from
@bubba/db/types. Although this is acceptable, ensure no duplication exists in other files. If these imports are heavily reused, consider a dedicated types file to streamline imports.
15-23: Consider isolatingFrameworkCategoryin a shared module.This new type extension is clean; however, it’s referenced in multiple components. To prevent drift or duplication, you might centralize the definition in a shared types file.
30-36: Avoid forcing a cast toOrganizationControlType[].The
.flatMap()call is strongly typed, but this manual casting can mask potential shape mismatches. Consider letting TypeScript infer the correct return type or refining the type to match the actual shape.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkOverview.tsx (2)
15-37: Be mindful of duplicated logic to check requirement completion.You repeat the same “published” checks in multiple places. If feasible, centralize the requirement-completion check in a shared utility to prevent maintenance headaches down the line.
45-57: Consider reusing shared logic for control compliance.Similar to checking requirement completion, consider factoring out the repeated logic into a helper function to avoid duplication and reduce the risk of divergences.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/actions/getFrameworkCategoriesAction.ts(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkControls.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkOverview.tsx(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/table/FrameworkControlsTableColumns.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/data/getFramework.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/data/getFrameworkCategories.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/lib/utils.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/page.tsx(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/actions/getOrganizationControl.ts(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/actions/getOrganizationControlProgress.ts(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/actions/getOrganizationControlRequirements.ts(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/components/SingleControl.tsx(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/components/table/ControlRequirementsTable.tsx(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/data/getControl.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/data/getOrganizationControlProgress.ts(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/hooks/useOrganizationControl.ts(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/hooks/useOrganizationControlProgress.ts(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/hooks/useOrganizationControlRequirements.ts(0 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/page.tsx(2 hunks)
💤 Files with no reviewable changes (8)
- apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/components/table/ControlRequirementsTable.tsx
- apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/actions/getFrameworkCategoriesAction.ts
- apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/hooks/useOrganizationControlRequirements.ts
- apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/actions/getOrganizationControl.ts
- apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/actions/getOrganizationControlRequirements.ts
- apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/hooks/useOrganizationControlProgress.ts
- apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/actions/getOrganizationControlProgress.ts
- apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/hooks/useOrganizationControl.ts
🧰 Additional context used
🧬 Code Definitions (5)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/lib/utils.ts (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/table/FrameworkControlsTableColumns.tsx (1) (1)
OrganizationControlType(22-38)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/page.tsx (4)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/data/getFrameworkCategories.ts (1) (1)
getFrameworkCategories(3-36)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/data/getFramework.ts (1) (1)
getFramework(3-21)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkOverview.tsx (1) (1)
FrameworkOverview(15-133)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkControls.tsx (1) (1)
FrameworkControls(30-59)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/page.tsx (2)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/data/getControl.ts (1) (1)
getControl(4-30)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/data/getOrganizationControlProgress.ts (2) (2)
getOrganizationControlProgress(18-88)ControlProgressResponse(6-16)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/components/SingleControl.tsx (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/data/getOrganizationControlProgress.ts (1) (1)
ControlProgressResponse(6-16)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkOverview.tsx (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkControls.tsx (1) (1)
FrameworkCategory(15-23)
🔇 Additional comments (18)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/data/getOrganizationControlProgress.ts (1)
6-16: Well-structured interface for progress dataThe
ControlProgressResponseinterface provides a clear contract for the progress data structure, making it easier to understand and work with the progress metrics throughout the application.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/page.tsx (2)
21-26: Good refactoring of error handling logicThe updated code properly handles both null results and error objects, ensuring users are redirected when data isn't available.
41-46: Props-based data passing is a good improvementPassing pre-fetched data as props to the SingleControl component simplifies the component hierarchy and removes the need for redundant hooks. This is a good architectural improvement.
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/controls/[id]/components/SingleControl.tsx (4)
13-13: Good type import for strong typingImporting the
ControlProgressResponsetype provides proper type safety for the component props.
25-25: Props interface update aligns with new data flow patternAdding the
organizationControlProgressproperty to the interface properly documents the component's requirements and ensures type safety.
28-41: Clean refactoring from hooks to props-based dataThe component now uses the passed progress data directly instead of fetching it with a hook. The
progressStatuscalculation logic is clear and handles all possible states.
43-43: Loading state check includes both required propsThe loading check now ensures both
organizationControlandorganizationControlProgressare available before rendering, which prevents potential null reference errors.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/lib/utils.ts (1)
11-21: Confirm that file URL usage and published fields align with the actual data model.
fileUrland thepublishedproperty are accessed directly, but they aren't explicitly visible in the provided type definition forOrganizationControlRequirement. Verify that these fields are indeed part of your schema. If they aren't guaranteed to exist, this may cause runtime errors or unexpected undefined behavior.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/page.tsx (3)
25-25: Check for multi-organization scenarios.Storing
organizationIdin the session is fine for single-org contexts. If a user can belong to multiple organizations, ensure the correctorganizationIdis selected or validated here.
40-44: Ensure consistent not-found handling.You're correctly redirecting if
organizationFrameworkis not found. Confirm that all calls togetFrameworkin the codebase follow a similar pattern so that users don't remain on a page that expects valid framework data.
48-55: Commendable prop-based data flow.Passing
organizationCategoriesandorganizationFrameworkas props toFrameworkOverviewandFrameworkControlsfosters clarity and reusability. Good job on removing redundant data-fetching hooks in the child components.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/table/FrameworkControlsTableColumns.tsx (1)
21-21: Make sure the new import aligns with all call sites.By removing the local
getControlStatusand importing it instead, you centralize the logic. Verify that the newly imported utility function retains the same signature, and ensure unit tests are added/updated to cover the new code location.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkControls.tsx (1)
25-28: Prop typing approach looks good.Passing
organizationCategoriesandframeworkIdas explicit props directly is more flexible than hooking into internal data fetching. This reduces hidden dependencies and clarifies the contract of this component.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkOverview.tsx (5)
3-3: New typed imports are correct.Importing
FrameworkandOrganizationFrameworkplusFrameworkCategoryfrom the local file is consistent. Just confirm those references are valid throughout your application.Also applies to: 9-9
11-12: Props interface now ensures required data is available.Providing
organizationCategoriesandorganizationFrameworkexplicitly makes it clear what data the component needs. This is a solid improvement over potentially implicit data fetching.
40-43: Clean compliance metric calculation.Calculating
compliancePercentageis straightforward with the ratio of completed vs. total. This approach is maintainable and easy to debug.
76-76: UI reads framework details directly from props.Displaying the framework’s name, description, and version from
organizationFrameworkclarifies data flow. This is consistent with the new props-based refactor.Also applies to: 80-80, 84-84
114-115: Dates are formatted locally; verify the user’s timezone needs.Using
format(…, "MMM d, yyyy")is good for readability. If user-specific timezones are required, consider adding locale/timezone support or clarifying that these are UTC dates.Also applies to: 123-124
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/data/getFrameworkCategories.ts (2)
15-15: Avoid usinganytype forstatusThe
statusproperty is typed asanywith a comment indicating it should be aComplianceStatusenum. Usinganybypasses TypeScript's type checking benefits.- status: any; // ComplianceStatus enum + status: ComplianceStatus; // Import this enum from wherever it's defined
24-49: Consider adding explicit return type to the functionThe function doesn't explicitly specify its return type, which would improve type safety and documentation.
export const getFrameworkCategories = async ( frameworkId: string, organizationId: string -) => { +): Promise<FrameworkCategories> => { const organizationCategories = await db.organizationCategory.findMany({ // ... }); return organizationCategories; };apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkControls.tsx (1)
34-36: Consider adding loading stateThe component returns null if organizationCategories is not provided, but it might be better to show a loading state or skeleton UI to improve user experience.
if (!organizationCategories) { - return null; + return <div className="flex justify-center p-4">Loading framework controls...</div>; }apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkOverview.tsx (2)
52-70: Code duplication in requirement status checkingThe logic to determine if a requirement is completed (lines 56-67) duplicates the logic used earlier (lines 26-37). Consider extracting this into a utility function.
+ const isRequirementCompleted = (req) => { + switch (req.type) { + case "policy": + return req.organizationPolicy?.status === "published"; + case "file": + return !!req.fileUrl; + case "evidence": + return req.organizationEvidence?.published === true; + default: + return req.published; + } + }; const totalRequirements = requirements.length; - const completedRequirements = requirements.filter((req) => { - switch (req.type) { - case "policy": - return req.organizationPolicy?.status === "published"; - case "file": - return !!req.fileUrl; - case "evidence": - return req.organizationEvidence?.published === true; - default: - return req.published; - } - }).length; + const completedRequirements = requirements.filter(isRequirementCompleted).length; // ... const compliantControls = allControls.filter((control) => { const controlRequirements = control.OrganizationControlRequirement; if (controlRequirements.length === 0) return false; - const completedControlRequirements = controlRequirements.filter((req) => { - switch (req.type) { - case "policy": - return req.organizationPolicy?.status === "published"; - case "file": - return !!req.fileUrl; - case "evidence": - return req.organizationEvidence?.published === true; - default: - return req.published; - } - }).length; + const completedControlRequirements = controlRequirements.filter(isRequirementCompleted).length; return completedControlRequirements === controlRequirements.length; }).length;
76-80: Consider adding fallback for optional chainingThe component uses optional chaining for organizationFramework properties, but doesn't provide fallback text if these properties are undefined.
- <CardTitle>{organizationFramework?.framework.name}</CardTitle> + <CardTitle>{organizationFramework?.framework.name || 'Unnamed Framework'}</CardTitle> <p className="text-sm text-muted-foreground"> - {organizationFramework?.framework.description} + {organizationFramework?.framework.description || 'No description available'} </p>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkControls.tsx(1 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkOverview.tsx(2 hunks)apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/data/getFrameworkCategories.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkControls.tsx (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/data/getFrameworkCategories.ts (1) (1)
FrameworkCategories(11-22)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkOverview.tsx (1)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/data/getFrameworkCategories.ts (1) (1)
FrameworkCategories(11-22)
🔇 Additional comments (8)
apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkControls.tsx (3)
8-11: Good use of strongly typed propsThe component now correctly uses the imported
FrameworkCategoriestype for the props, making the component more type-safe.
13-16: Props refactoring improves component architectureThe component now accepts pre-fetched data as props rather than fetching it internally, which is a best practice for separation of concerns and improves testability.
20-31: Improved naming clarity withorgControlRenaming the variable from
controltoorgControlmakes the code more readable by avoiding confusion between the organization control object and the nested control property.apps/app/src/app/[locale]/(app)/(dashboard)/[orgId]/(home)/overview/frameworks/[frameworkId]/components/FrameworkOverview.tsx (5)
10-13: Props typing improved with detailed typesThe props interface now clearly specifies the expected data structure with proper typing, enhancing type safety and making the component's dependencies explicit.
15-18: Component now receives pre-fetched dataRefactoring to accept pre-fetched data as props follows best practices for separation of concerns and improves testability.
19-38: Well-structured compliance calculation logicThe calculation of compliance metrics is well-organized and handles different requirement types appropriately. Good use of flatMap to process nested arrays.
39-43: Prevent division by zero with guard clauseGood defensive programming by checking if totalRequirements is greater than zero before calculating the percentage.
114-116: Good handling of date formatting with fallbacksThe component correctly handles potential undefined dates by providing fallback text when dates are not available.
Also applies to: 123-125
| const organizationCategories = await db.organizationCategory.findMany({ | ||
| where: { | ||
| organizationId, | ||
| frameworkId, | ||
| }, | ||
| include: { | ||
| organizationControl: { | ||
| include: { | ||
| control: true, | ||
| OrganizationControlRequirement: { | ||
| include: { | ||
| organizationPolicy: true, | ||
| organizationEvidence: true, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for database query
The database query lacks error handling. If the query fails, the error will propagate up the call stack without any context about what operation failed.
export const getFrameworkCategories = async (
frameworkId: string,
organizationId: string
) => {
+ try {
const organizationCategories = await db.organizationCategory.findMany({
where: {
organizationId,
frameworkId,
},
include: {
organizationControl: {
include: {
control: true,
OrganizationControlRequirement: {
include: {
organizationPolicy: true,
organizationEvidence: true,
},
},
},
},
},
});
return organizationCategories;
+ } catch (error) {
+ console.error(`Failed to fetch framework categories: ${error}`);
+ throw new Error(`Failed to fetch framework categories for framework ${frameworkId} and organization ${organizationId}`);
+ }
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const organizationCategories = await db.organizationCategory.findMany({ | |
| where: { | |
| organizationId, | |
| frameworkId, | |
| }, | |
| include: { | |
| organizationControl: { | |
| include: { | |
| control: true, | |
| OrganizationControlRequirement: { | |
| include: { | |
| organizationPolicy: true, | |
| organizationEvidence: true, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }); | |
| export const getFrameworkCategories = async ( | |
| frameworkId: string, | |
| organizationId: string | |
| ) => { | |
| try { | |
| const organizationCategories = await db.organizationCategory.findMany({ | |
| where: { | |
| organizationId, | |
| frameworkId, | |
| }, | |
| include: { | |
| organizationControl: { | |
| include: { | |
| control: true, | |
| OrganizationControlRequirement: { | |
| include: { | |
| organizationPolicy: true, | |
| organizationEvidence: true, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }, | |
| }); | |
| return organizationCategories; | |
| } catch (error) { | |
| console.error(`Failed to fetch framework categories: ${error}`); | |
| throw new Error( | |
| `Failed to fetch framework categories for framework ${frameworkId} and organization ${organizationId}` | |
| ); | |
| } | |
| }; |
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores